fix(core): tolerate AlreadyExists in FSUtil.ensureDir#36542
Conversation
github-actions
Bot
added
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Recursive makeDirectory should be idempotent, but some Windows runtimes surface AlreadyExists for a directory that already exists. Since v1.17.15, Config.ensureGitignore calls fs.ensureDir on every resolved config directory, so startup crashed with 'Unexpected server error' whenever a project's .opencode directory already existed. ensureDir now treats AlreadyExists as success when the path is already a directory, while still failing when a file occupies the path. Fixes anomalyco#35828
BB-84C
force-pushed
the
ensuredir-alreadyexists
branch
from
b1d79f8 to
7003a09
Compare
|
apologies, playing around w/ ur pr |
@rekram1-node No problem at all, thanks for your time with this PR! |
rekram1-node
force-pushed
the
ensuredir-alreadyexists
branch
from
c229056 to
e82a069
Compare
|
Reduced this back to the minimum production fix and removed the added regression-test scaffolding. I am not retaining a native Windows regression test because the Windows CI probe passed with the workaround omitted, so that test did not reproduce or validate the reported failure. A synthetic filesystem that always returns |
@rekram1-node Thank you! |
2 participants
Issue for this PR
Closes #35828
Type of change
What does this PR do?
v1.17.15 added
fs.ensureDir(dir)toConfig.ensureGitignore, which runs for every resolved config directory duringConfig.loadInstanceState, and the call site is wrapped inEffect.orDie.FSUtil.ensureDircallsmakeDirectory(path, { recursive: true })with no error handling.Recursive
makeDirectoryis supposed to be idempotent, but on some Windows runtimes it reportsAlreadyExistsfor a directory that already exists. So once a project had a.opencodedirectory, startup died with "Unexpected server error" (works on 1.17.14, broke on 1.17.15).The fix makes
ensureDirtreatAlreadyExistsas success, but only when the path is actually a directory — a file sitting at that path still fails. This mirrors theNotFoundcatch already used a few lines down inwriteWithDirs, so behavior only changes for the exact reported case.How did you verify your code works?
AlreadyExistsresult frommakeDirectory: it fails without this patch and passes with it.ensureDirstill fails when a file occupies the path.packages/corefilesystem suite passes (29 tests) andbun typecheckforpackages/coreis clean.I couldn't reproduce the crash on my own machine — recursive mkdir is idempotent on my Bun build — which is why the regression test injects the error directly instead of depending on the platform quirk.
Screenshots / recordings
N/A — not a UI change.
Checklist